Repository navigation
feat(sandbox): count running sandboxes in a gym.sandbox.active up-down counter - #3581
Closed
iguehring-nvidia wants to merge 2 commits into
Closed
iguehring-nvidia wants to merge 2 commits into
iguehring-nvidia wants to merge 2 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
iguehring-nvidia
marked this pull request as ready for review
September 22, 2026 15:56
…n counter Gym had no "sandboxes running now" signal: the sandbox span group emits gym.sandbox.start and gym.sandbox.exec spans but no stop span and no gauge, so dashboards had to approximate the count from in-flight /run requests on the agent server. AsyncSandbox now adds +1 to gym.sandbox.active when a provider hands back a handle and -1 when Gym releases it (also when the provider's close fails or the initial file upload fails), dimensioned by nemo.gym.sandbox.provider. An up-down counter rather than a gauge so a backend sums correctly across the agent-server processes that each hold a share of the sandboxes. A connect()ed sandbox is not subtracted, since the starting process counted it. The instrument lives on the lens meter but is Gym-owned; it follows the existing sandbox span-group gate and is a no-op without telemetry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com> (cherry picked from commit 2e188aa)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
iguehring-nvidia
force-pushed
the
iguehring/sandbox-active-counter
branch
from
September 22, 2026 16:01
aada9dc to
7333804
Compare
Contributor
Author
|
Folded into #3633, which carries these two commits unchanged plus the rest of the sandbox lifecycle telemetry (durations, sandbox ids, stop span, retry counter) as one standalone PR against main. Closing to avoid reviewing the counter twice. |
JeffPengCoder
pushed a commit
to JeffPengCoder/Gym
that referenced
this pull request
Sep 23, 2026
…nd a stop span (NVIDIA-NeMo#3633) Supersedes NVIDIA-NeMo#3581 (its two commits are the first two here) and takes the sandbox part of NVIDIA-NeMo#2935, with @oyilmaz-nvidia as co-author on the hunks from there. Agreed with Onur as the split of NVIDIA-NeMo#2935 that this side owns. ## What One coherent "sandbox lifecycle telemetry" set in `AsyncSandbox` and the providers, all under the existing `sandbox` span group, all attributed by `nemo.gym.sandbox.provider`: | Instrument | Kind | Where | Origin | |---|---|---|---| | `gym.sandbox.active` | up-down counter: +1 when a provider hands back a handle, −1 when Gym releases it | facade `start()` / `stop()` | this PR (was NVIDIA-NeMo#3581) | | `gym.sandbox.startup_duration_ms` | histogram | facade `start()` | NVIDIA-NeMo#2935 | | `gym.sandbox.exec_duration_ms` | histogram | facade `exec()` | NVIDIA-NeMo#2935 | | `gym.sandbox.create_retry_total` | counter, one per retried create attempt | OpenSandbox and Daytona retry callbacks | NVIDIA-NeMo#2935 (Daytona), new (OpenSandbox) | | `nemo.gym.sandbox.id` | span attribute on start / exec / stop | facade | NVIDIA-NeMo#2935 | | `gym.sandbox.stop` | span around `provider.close` | facade `stop()` | new | Supporting change: `nemo_gym/telemetry/gym_metrics.py`, a small registry for attributed instruments (per-meter cache, histogram / counter / up-down-counter recorders, no-op without telemetry, never raises), taken from NVIDIA-NeMo#2935. `record_sandbox_active` lives there; `telemetry/metrics.py` keeps forwarding to nemo-lens's five fixed instruments and points here for anything attributed. ## Why Gym's sandbox API emitted `gym.sandbox.start` and `gym.sandbox.exec` spans and nothing else: no stop span, no metric. Three questions a dashboard could not answer: how many sandboxes exist right now (dashboards approximated it from in-flight `/run` requests, which undercounts by more than two times on Terminal-Bench 4, where a session holds an agent sandbox, a shared-logs helper and later a verifier sandbox); how long provisioning and commands take as a metric rather than only as spans, which need a span-metrics connector somewhere to become one; and how often creation has to be retried. On a degraded provider the last one is the difference between "39 rollouts dropped" and "101 retries, then 39 dropped". Design points: - **Up-down counter, not a gauge**, for `gym.sandbox.active`: several server processes each hold a share of the sandboxes; a backend sums counters correctly across processes, while a gauge shows whichever process wrote last. −1 happens once the provider has actually released the sandbox: a `close()` the provider rejected has released nothing, so the sandbox stays started and counted and a later `stop()` retries it (the compose group depends on that retry). A `connect()`ed sandbox is neither counted nor subtracted, since the starting process counted it. - **Explicit bucket boundaries up to thirty minutes** on both duration histograms. The SDK default ends at 10 s; a remote sandbox start takes about a minute, so every sample would otherwise land in the overflow bucket and a p95 would read "10 s" whatever the truth. (NVIDIA-NeMo#2935 used the defaults; fixed while consolidating.) - **Retry counting sits in the providers** because retrying is provider-internal: the facade calls `create()` once. The two providers that retry (OpenSandbox, Daytona) record from their tenacity `before_sleep` callback, next to the warning they already log, so the counter can never disagree with the log. - **The stop span** wraps only `provider.close`; a failing close yields an error-status stop span and leaves the count unchanged until the retry succeeds. Deliberately not in this PR (they belong to other splits of NVIDIA-NeMo#2935): the CPU / memory / GPU sampling at span close, and the `TimedSemaphore` swap in the providers. ## Validation - `pytest tests/unit_tests/telemetry tests/unit_tests/test_sandbox.py tests/unit_tests/test_opensandbox_provider.py`: pass locally (nemo-lens-gated span tests skip locally and run on the cluster venv). Tests, all against a real in-memory OTel reader: - `test_sandbox_active.py` (10): start then stop moves 1 → 0 and a second stop stays 0; failed create counts nothing; failed initial upload releases the count; a failed close keeps the sandbox counted until a retried stop releases it; group off records nothing; `connect()`ed sandbox not subtracted; no-op without telemetry; errors never reach the caller; non-monotonic and dimensioned by provider; one instrument per meter. - `test_sandbox_lifecycle_metrics.py` (4): one startup sample per start and one exec sample per command with the provider attribute; explicit bounds end at 1,800,000 ms; failed create records no startup sample; group off records nothing. - `test_sandbox_create_retry.py` (3): both providers' callbacks count by provider when the group is on, silent when off. - `test_call_sites.py` (+1 test, +3 assertions): `nemo.gym.sandbox.id` on start and exec spans; a `gym.sandbox.stop` span with provider and id. - `pre-commit run --files ...` on the changed files: pass. ## Rollout evidence Slurm cluster, Terminal-Bench 4 (mini-SWE), OpenSandbox provider, `sandbox` span group on, metrics exported through an OTLP collector that also derives span metrics, so every Gym-emitted number has an independent counterpart to check against. **8-task run (concurrency 8, 24 min, all 8 rollouts scored, collector shut down cleanly):** | Instrument | Gym-emitted | Independent source | Result | |---|---|---|---| | `gym.sandbox.create_retry_total{provider="opensandbox"}` | 10 | 10 "Retrying OpenSandbox sandbox create" lines in the driver log | exact match | | `gym.sandbox.startup_duration_ms` | 25 samples, mean 91.6 s, p50 ≤ 60 s, p95 ≤ 300 s | span-derived `gym.sandbox.start` duration: 25 samples, mean 91.6 s, same p50 / p95 | identical | | `gym.sandbox.exec_duration_ms` | 580 samples, mean 2.4 s, p95 ≤ 5 s | span-derived `gym.sandbox.exec`: 581 successful + 1 error span, p95 ≤ 5 s | agree (see note) | | `gym.sandbox.stop` span | 25 spans, all OK | 25 starts | every sandbox released | | `gym.sandbox.active` | 0 → peak 14 → 0 | 8 sessions, each holding an agent sandbox, a shared-logs helper and later a verifier sandbox | consistent, no leak | With the SDK's default buckets, the startup histogram (mean 92 s) would have placed every sample in the overflow bucket and reported "10 s" for any percentile; with the explicit boundaries the p50 / p95 land in the 60 s / 300 s buckets, matching the span-derived series bucket for bucket. Note on the exec count: the duration histograms are recorded after the span closes, so an operation that raises never reaches the record and is absent from the histogram; it does appear in the span-derived series with `STATUS_CODE_ERROR`. The one error exec accounts for the gap. This is deliberate: a failure's duration is not a latency, and the failure itself is counted where the status lives. **66-task run at concurrency 16 (earlier, counter only):** `gym.sandbox.active` rose from 8 to a peak of 39 while the in-flight-rollouts proxy reported 16, and returned to 0 through 6 `SANDBOX_NOT_FOUND` rebuilds and 3 failed starts. ## Compatibility Additive. Nothing is recorded unless telemetry is on and the `sandbox` span group is enabled. New module `telemetry/gym_metrics.py`; no public API changes. `AsyncSandbox.stop()` semantics are unchanged from main: a failed `close()` leaves the sandbox started so the call can be retried (an earlier revision of this PR broke that and failed `test_docker_compose`; fixed in the last commit). --------- Signed-off-by: Ingo Guehring <iguehring@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Onur Yilmaz <oyilmaz@nvidia.com>
soluwalana
pushed a commit
to soluwalana/Gym
that referenced
this pull request
Sep 28, 2026
NVIDIA-NeMo#1558) ## Summary - Consume a complete token bundle from `choice.message` without making a second `/tokenize` request. - Support opt-in vLLM response token IDs for endpoints that return top-level `prompt_token_ids` and choice-level `token_ids`. - Retain `/tokenize` for integrations that return neither inline representation. - Reject partial or conflicting sources. Token capture supports one completion choice per request (`n` omitted or set to `1`). ## Why Gym needs the prompt token IDs, generated token IDs, and selected-token log probabilities used during generation. The previous adapter reconstructed prompt IDs through a second awaited `/tokenize` request. That request adds inter-turn latency and can reproduce a different prompt when prompt-affecting inputs are omitted. NeMo RL [NVIDIA-NeMo#3390](NVIDIA-NeMo/RL#3390) attaches the token bundle to the assistant message. Gym reads that bundle directly and does not request duplicate response-level IDs by default. Gym cannot require that message shape from every training integration. Other integrations, including Verl, can request vLLM's response token IDs or continue using the compatibility fallback. ## Source precedence 1. A complete token bundle on `choice.message`. 2. Top-level `prompt_token_ids`, choice-level `token_ids`, and `choice.logprobs` from the vLLM response. 3. Generation metadata from `choice.logprobs` and prompt IDs from `/tokenize`. `request_prompt_and_generation_token_ids: true` enables the second source by sending `return_token_ids=true` to compatible vLLM endpoints. The option defaults to `false`. If both inline sources are present, their token IDs must agree. The selected training message is validated once to avoid repeated list scans and copies at long sequence lengths. The fallback forwards `model`, `messages`, `tools`, `chat_template_kwargs`, `mm_processor_kwargs`, and `required_prefix_token_ids`. This keeps fallback tokenization aligned with the generation prompt. `prompt_logprobs` is not requested. vLLM returns prompt IDs independently when `return_token_ids` is enabled, and Gym does not consume prompt-token log probabilities. ## Related work - Gym [NVIDIA-NeMo#700](NVIDIA-NeMo#700) is superseded by this change. - Gym [NVIDIA-NeMo#2576](NVIDIA-NeMo#2576) is superseded by this change. - NeMo RL [NVIDIA-NeMo#3390](NVIDIA-NeMo/RL#3390) produces the message-level token bundle consumed by the first source. - NeMo RL [NVIDIA-NeMo#3581](NVIDIA-NeMo/RL#3581) reuses replayed token metadata across turns. - Gym [NVIDIA-NeMo#1784](NVIDIA-NeMo#1784) can add the Dynamo `nvext.engine_data` source independently. - Gym [NVIDIA-NeMo#2324](NVIDIA-NeMo#2324) can retain its video-specific schema, conversion, and adapter behavior independently. --------- Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
2 tasks done
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
gym.sandbox.active, an OpenTelemetry up-down counter of sandboxes a process currently holds, dimensioned bynemo.gym.sandbox.provider.AsyncSandbox.start()adds +1 once the provider has handed back a handle;stop()subtracts 1 in afinally, so a failing providerclose()or a failed initial file upload cannot leave the count high. A sandbox rebuilt in another process viaconnect()is neither counted nor subtracted. The counter follows the existingsandboxspan-group gate and is a no-op without telemetry.Why
The sandbox span group emits
gym.sandbox.startandgym.sandbox.execspans but no stop span and no gauge, so "how many sandboxes are running right now" could only be approximated from in-flight/runrequests on the agent server. That proxy assumes one sandbox per rollout for the rollout's whole life. Terminal-Bench 4 breaks it: a session holds an agent sandbox plus a shared-logs helper and, later, a verifier sandbox.An up-down counter rather than a gauge because several server processes each hold a share of the sandboxes; a backend sums counters correctly across processes, while a gauge would show whichever process wrote last (the concern
nemo_gym/telemetry/metrics.pyalready documents forgym.servers.active). The instrument is created on the lens meter from Gym so no lens release is needed.No issue filed; this is a small, self-contained instrumentation gap found while building the orchestrator observability path (follow-up PRs).
Validation
pytest tests/unit_tests/telemetry/test_sandbox_active.py tests/unit_tests/test_sandbox.py tests/unit_tests/telemetry/test_metrics.py tests/unit_tests/telemetry/test_fallbacks.py: pass (the new file asserts against a real in-memory OTel metric reader; start/stop, failed create, failed upload, failing close, group off, connect()).pre-commit run --files nemo_gym/sandbox/api.py nemo_gym/telemetry/metrics.py tests/unit_tests/telemetry/test_sandbox_active.py: pass.Rollout evidence
Terminal-Bench 4 (mini-SWE, 66 tasks, OpenSandbox provider,
tb4_concurrency: 16) on a Slurm cluster with thesandboxspan group on and metrics exported through an OTLP collector: the counter rose from 8 to a peak of 39 running sandboxes while the in-flight-rollouts proxy reported 16, then returned to 0 at the end of the run through 6SANDBOX_NOT_FOUNDrebuilds and 3 failed starts. Stored asgym_sandbox_active(annotation unit, no suffix appended).Compatibility
Additive. No config change; nothing is recorded unless telemetry is on and the
sandboxspan group is enabled.